@claude review

Claude finished @cliffhall's task in 2m 28s —— View job

PR Review: feat(everything): add tool annotations to all tools

• Gather context and understand the PR
• Read changed files to examine implementation
• Review annotation correctness and rationale
• Check consistency with other servers
• Post review findings

Overall: ✅ Looks good — well-reasoned, complete coverage, consistent with the pattern in other servers.

This is a clean, focused PR. All 17 registered tools receive explicit annotations, the rationale table in the PR description is sound, and it correctly excludes simulate-research-query (which uses registerToolTask with a different experimental config shape). A few minor observations below.

Minor observations

1. trigger-long-running-operation: readOnlyHint: true + idempotentHint: false is semantically in tension

src/everything/tools/trigger-long-running-operation.ts:20-25

annotations: {
  readOnlyHint: true,
  destructiveHint: false,
  idempotentHint: false,   // ← in tension with readOnlyHint: true
  openWorldHint: false,
},

Per the MCP spec, a readOnlyHint: true tool does not modify its environment — which generally implies idempotency (calling it N times with the same args leaves the environment in the same state as calling it once). Setting idempotentHint: false alongside readOnlyHint: true may confuse clients that infer idempotency from the read-only flag.

The most internally consistent choice would be either:

• readOnlyHint: true, idempotentHint: true — this is strictly accurate since the tool produces identical output and makes no persistent state changes, or
• readOnlyHint: false, idempotentHint: false — if you consider time-consuming notifications as a non-trivial side effect.

The PR's current choice is defensible ("each invocation runs its own independent operation"), but the combination is worth documenting or reconsidering for clarity in a reference server.

2. gzip-file-as-resource: openWorldHint: true is correct, but worth noting the conditional nature

src/everything/tools/gzip-file-as-resource.ts:51-56

The tool accepts both external HTTP/HTTPS URLs and data: URIs. When a data: URI is passed it has no open-world interaction; openWorldHint: true is the safe and correct choice here (the default input is an external GitHub URL), but a comment in the description or code noting this could help readers understand why a "file compressor" is flagged open-world.

3. All tools with readOnlyHint: true also explicitly set destructiveHint: false — good practice for a reference server

This is intentional and appropriate. Even though readOnlyHint: true implies non-destructiveness, being explicit is the right call for a showcase server. No issue here.

4. simulate-research-query exclusion is correct

registerToolTask (experimental tasks API) takes a different config object — there's no annotations key at the top level in that shape. The PR is right to leave it out. If/when that API gains annotation support, a follow-up PR can address it.

Summary

The annotation choices are accurate and well-considered. The only substantive point worth discussing before merge is #1 (trigger-long-running-operation's readOnlyHint: true + idempotentHint: false). Everything else is either correct as-is or a stylistic note. No functional regressions — TypeScript passes and all 95 tests remain green.

Good catch on trigger-long-running-operation — updated in 6369f24.

Changed readOnlyHint: true → readOnlyHint: false. The tool sends progress notifications as side effects on every invocation, which means it's not truly read-only. readOnlyHint: false + idempotentHint: false is now internally consistent and more accurate for a reference server.

All 95 tests still passing.